Adding qualifiers to MetaEdge fixes #342 #387
Conversation
…ge of regex validation
…onerAPI into mkg_qualifiers
…g against updated schema: see reasoner-validator issue 56
|
@edeutsch (cc: @vdancik @cbizon), I am highly supportive of this (@sierra-moxon's) PR. TRAPI implementations all actually need to curate their Quick implementation of this PR, even as a patch release to the latest TRAPI, would accelerate testing progress. |
|
@edeutsch I might be confused, but didn't you show me the other day that this is already part of the spec somewhere? |
|
@cbizon, my recollection is that we were talking about knowledge_type. TRAPI 1.3 can already indicate in the MetaEdge whether knowledge_type inferred is supported: ReasonerAPI/TranslatorReasonerAPI.yaml Line 1008 in c5d101a |
|
@sierra-moxon would you base this PR on branch 1.4 where all the new work is going on (not master)? |
|
also a note to @mbrush who was planning on making a PR: please base all PRs for TRAPI 1.4 additions to the "1.4" branch, not master, thanks! |
|
You are correct - got my wires crossed! |
.yamllint.yml
Outdated
| @@ -5,7 +5,7 @@ rules: | |||
| empty-values: | |||
| forbid-in-block-mappings: true | |||
| line-length: | |||
| max: 79 | |||
| max: 100 | |||
There was a problem hiding this comment.
This will cause an overrun on my VT100!
There was a problem hiding this comment.
ok I can change it back :). Some of our descriptions are long enough that I was getting linting errors for line length (and linting errors about trailing spaces when I tried to break the description up into multiple lines) which felt a bit too constrictive. But I don't feel strongly. :)
It would be really nice to be able to run tests in this repo too, we have an open issue about that with reasoner-validator team. NCATSTranslator/reasoner-validator#56
…ociation can help the test harness validate qualifiers.
capasfield
requested review from
edeutsch,
RichardBruskiewich,
vdancik,
andrewsu,
brettasmi,
edgargaticaCU,
kennethmorton,
marcdubybroad and
MarkDWilliams
TranslatorReasonerAPI.yaml
Outdated
| items: | ||
| type: string | ||
| example: [expression, activity, abundance, degradation] | ||
| nullable: true |
There was a problem hiding this comment.
This nullable:true seems to be nested under items? Implying that null items can be in the list? Perhaps this should be de-indented so that applicable_values is the nullable:true thing?
MetaQualifierobjects on theMetaEdgeobject. The big question here is whether or not this representation gives enough information to both ARAs and QC to validate and query the KP correctly.associationproperty toMetaEdgeto store thebiolink:Associationclass that eachMetaEdgeshould comply with. I propose making this property required, as validation will be much easier with a declared association. However, we could instead make this property optional and only validate that edge is a validbiolink:Association(for example, that it has subject, predicate, object, qualifiers of the appropriate biolink types) when it is provided. Is there a preference for either of these options?other: